-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
plume prune enhancement #343
Conversation
Here's the log output for a dry-run on the edge channel: |
6d97932
to
a02cda8
Compare
Needed to access "lastLaunchedTime" attribute on AMIs.
a02cda8
to
51a9e08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicks only.
23e480a
to
f090429
Compare
@tormath1 would love it if you would take a look as well |
@@ -56,6 +67,18 @@ func runPrune(cmd *cobra.Command, args []string) { | |||
if len(args) > 0 { | |||
plog.Fatal("No args accepted") | |||
} | |||
if daysLastLaunched < 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the helper:
Only used when --check-last-launched is set.
Even if the default value is 0, we could add an extra condition like you're doing later:
if daysLastLaunched < 0 { | |
if checkLastLaunched && daysLastLaunched < 0 { |
EDIT: we could even group the two statements:
if daysLastLaunched < 0 { | |
if checkLastLaunched && (daysLastLaunched < 0 || daysLastLaunched == 0) { | |
daysLastLaunched = days |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would rather keep the if-blocks separate so that we can report a clear error messages to users. But you made me realize I need at least one more check: if !checkLastLaunched && daysLastLaunched > 0
so that we tell users that the option will be ignored.
Images are first marked "soft-deleted" by removing launchpermissions from them. If a "soft-deleted" image is found again, it is permanently deleted after a grace period expires. Soft-deleting has the advantage that we do not invalidate ami ids.
f090429
to
d698ec3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! :)
plume prune enhancements
How to use
Testing done
Tested with dry-run only, would run on "edge" channel first.
changelog/
directory (user-facing change, bug fix, security fix, update)